Feature: add new formatters for sponsors#497
Conversation
andrestejerina97
force-pushed
the
feature/add-formatter-for-sponsors
branch
from
7202e6a to
e4a2973
Compare
andrestejerina97
force-pushed
the
feature/add-formatter-for-sponsors
branch
from
e4a2973 to
c885e10
Compare
|
@andrestejerina97 |
martinquiroga-exo
left a comment
martinquiroga-exo
left a comment
There was a problem hiding this comment.
@andrestejerina97 please see comments.
andrestejerina97
force-pushed
the
feature/add-formatter-for-sponsors
branch
2 times, most recently
from
6f02211 to
893af02
Compare
martinquiroga-exo
left a comment
martinquiroga-exo
left a comment
There was a problem hiding this comment.
LGTM. Needs a rebase in order to fix conflicts.
andrestejerina97
force-pushed
the
feature/add-formatter-for-sponsors
branch
3 times, most recently
from
9853be1 to
0bfb5ba
Compare
Signed-off-by: Jose Andres Tejerina <andres.tejerina@exomindset.co>[C
andrestejerina97
force-pushed
the
feature/add-formatter-for-sponsors
branch
from
0bfb5ba to
a4e0e0a
Compare
| $this->assertStringContainsString((string)self::SPONSOR_ID, $result); | ||
| $this->assertStringContainsString('rate', $result); |
There was a problem hiding this comment.
@andrestejerina97 The string "rate" does not appear anywhere in this output actually the test are not run on the CI
please correct it here
.github/workflows/push.yml
So the failing assertStringContainsString('rate', $result) assertion in SponsorSummitRegistrationDiscountCodeFormatterTest
would never be caught by CI. The formatter tests exist but are not included in the CI pipeline.
There was a problem hiding this comment.
FYI i just include these OTEL test at CI and now are breaking please rebase against main and do check
https://github.com/OpenStackweb/summit-api/actions/runs/22197651105/job/64202064146
| switch ($this->event_type) { | ||
| case IAuditStrategy::EVENT_ENTITY_CREATION: | ||
| return sprintf( | ||
| "Sponsor User Info Grant (ID: %s) granting access to user '%s' for Sponsor %s created by user %s", |
There was a problem hiding this comment.
@andrestejerina97 There are two spaces between %s and created. This is a typo
| ], | ||
| \models\summit\Sponsor::class => [ | ||
| 'enabled' => true, | ||
| 'strategy' => \App\Audit\ConcreteFormatters\SponsorAuditLogFormatter::class, |
There was a problem hiding this comment.
@andrestejerina97 neither SponsorAuditLogFormatter nor SummitEventTypeAuditLogFormatter has any tests
smarcet
left a comment
smarcet
left a comment
There was a problem hiding this comment.
@andrestejerina97 please review
4 participants
ref: https://app.clickup.com/t/86b7wynmm